Skip to content

Feat/payment state durability - #110

Open
1amKhush wants to merge 8 commits into
ContextVM:masterfrom
1amKhush:feat/payment-state-durability
Open

1amKhush wants to merge 8 commits into
ContextVM:masterfrom
1amKhush:feat/payment-state-durability

Conversation

@1amKhush

@1amKhush 1amKhush commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Related: #109. Depends on #108.

Summary

Prevent duplicate payment offers and stale authorization changes using canonical invocation identity, atomic admission/claim operations, generation fencing, and retained transparent-request replay records.

  • Expose PaymentStateStore for application-provided backends; bundle only InMemoryPaymentStateStore, with no added storage dependencies, database export, migration, or database CI service.
  • Document that one paying server identity must run in one process with a shared store. The default store loses invoices, grants, and replay records on restart.
  • Share non-empty, 2,048-character PMI validation across client/server boundaries. Preserve custom names and HTTPS identifiers using exact matching.
  • Return an actionable reconciliation message for expired paid grants without issuing another invoice or restoring execution rights.
  • Add the missing breaking-change changeset and preserve the reviewed state-machine phases, replay deadlines, and generation fences.

Compatibility

AuthorizationStore replaces its synchronous grant/claim/pending/clear API with asynchronous admit, transition, and read. Configure capacity and replay admission through PaymentStatePolicy; inject a shared instance through ServerPaymentsOptions.stateStore.

The default store loses state on process/store restart. Persistence and cross-process coordination require an application-provided implementation; the application owns dependencies, initialization, atomicity, and shutdown. Backend errors propagate without fallback to fresh memory. PostgreSQL PR #116 is closed and is not a prerequisite.

Unclaimed expired grants remain retained pending explicit recovery/retention policy (#114). Server outcomes and result recovery follow in #112.

Validation

  • Core concurrent suite, without PostgreSQL: 786 passed, 5 live-wallet tests skipped, 0 failures.
  • Typecheck, lint, build, export checks, frozen lockfile, Node 18 API/state smoke, and browser bundles passed.
  • New PMI regressions and the expired-grant message assertion reproduced failures before the fixes.
  • Custom-interface backend failure tests verify that storage errors cannot mint an invoice or dispatch a handler.
  • Updated server guide: 71-page docs build passed; published separately in the docs repository.

No live-wallet settlement runs were performed. Work notes and ticket drafts are excluded from Git.

Copilot AI balanced review requested due to automatic review settings October 18, 2026 18:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@ContextVM-org

Copy link
Copy Markdown
Contributor

Verified locally beyond the PR's claims: full suite against a live Postgres 17.7 (784 pass / 0 fail, matching CI), lint/typecheck clean, and we confirmed the load-bearing behaviors empirically — concurrent two-worker transparent delivery produces exactly one payment_required and no spurious payment_rejected; replay after lease expiry + worker restart dedups across all three encryption modes. The mode asymmetry (event-keyed tombstones for transparent, authorization-keyed coalescing for explicit), generation fencing, JCS canonical identity with published vectors, and monotonic clock guards are all exactly right. Also verified the phase machine is near-minimal: we tried to collapse phases/leases and each deletion reopens a #109 hole.

One structural decision before merge, plus smaller items:

1. Defer the PostgreSQL backend — split the PR. The state machine + in-memory store fixes every #109 bug within a process (~1,300 lines). Postgres (+CI service, migrations, pg peer-dep, ~300 src/~600 test lines) buys only two things: restart durability and cross-worker atomicity. Today's expected loss without it is roughly (crash frequency) × (payments in a 5-min lease window) × (amount) — cents per year at current volumes. Per YAGNI we shouldn't carry the maintenance surface until a deployment actually runs one paying identity across multiple processes. The cut is surgical thanks to the PR's own seams: src/payments/postgres/, the pg-only e2e, the CI postgres service, the export entry, the peer-deps. Keep the backend on a ready branch — the contract suite makes re-landing it later a clean merge, not a rewrite.

2. Document the constraint the split creates: one paying server identity = one process. Without that sentence in the withServerPayments JSDoc + how-to, someone will scale horizontally and reintroduce the exact #109 bugs invisibly.

3. Ticket: expired-grant retention horizon. A paid grant that expires unclaimed (user pays, misses the 5-min authorizationTtlMs, returns) blocks its invocation hash with -32000 payment_expired forever — retainUntilMs: null, never collected, no recovery method (verified empirically). With pg deferred this self-heals on restart, so it's no longer release-blocking — but fix it properly (collect after grantExpiry + X) in the durable-backend PR, and meanwhile consider an actionable error message.

4. Still needed on the core: the PMI grammar decision is now centralized in payment-option-schema.ts (good — that heals the #108 client/server asymmetry) but it must be decided consciously: kebab-only as a normative CEP-8 restriction, or the full W3C grammar including https:// identifiers — plus .max(2048). And add the missing changeset — this breaks the public AuthorizationStore API. Ticket the docs item (paymentTtlMs must be ≥ processor invoice TTL, or late payments escape verification) and the observability pass (admission/transition outcomes are silent server-side).

One deliberate divergence worth a code comment: payment records use Map + deadlines instead of the codebase-wide LruCache — correct (recency is the wrong eviction axis for replay tombstones), but note it so nobody "fixes" it back.

Verdict: request changes for 1 (split), 2, and 4 — all cheap. The core is the right foundation; keep it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants